Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache org client and remove solana cache invalidation for invalids #580

Merged
merged 6 commits into from
Jul 28, 2023

Conversation

maplant
Copy link
Contributor

@maplant maplant commented Jul 26, 2023

This should dramatically speed up the iot packet verifier in the pathological case where we've disabled and org, but it continues to hammer the packet verifier. This is basically the worst case; insufficient funds means we must check balance sufficiency every time a packet comes in. We also cache the org disabling so that we don't send outbound requests to the org client every time this happens.

TODO:

  • Fix remaining integration test

Copy link
Contributor

@andymck andymck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is my understanding correct that should an org be disabled and subsequently cached it will remain disabled until whenever next tick of monitor_funds shows it is back in credit again and thus packets can potentially be declared invalid if the owner has credited the account but that hasnt yet been picked up by monitor funds ?

Also I see a bunch of unwraps in this code. Are we dropping the hard rule of no unwraps? In the past even if the code was infallible there were to be avoided

@maplant
Copy link
Contributor Author

maplant commented Jul 27, 2023

Is my understanding correct that should an org be disabled and subsequently cached it will remain disabled until whenever next tick of monitor_funds shows it is back in credit again and thus packets can potentially be declared invalid if the owner has credited the account but that hasnt yet been picked up by monitor funds ?

Yes, exactly. Effectively this is to prevent an account from continuing to send invalid packets despite us disabling them. We'll increase how often we check for updated funds to compensate

Also I see a bunch of unwraps in this code. Are we dropping the hard rule of no unwraps? In the past even if the code was infallible there were to be avoided

I've always maintained that for hashmaps using unwraps is fine if you know the entry is populated. It's not worth throwing an error that can never occur

@maplant maplant merged commit 61f92e7 into main Jul 28, 2023
1 check passed
@maplant maplant deleted the map/dont-update-cache branch July 28, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants